[compiler-rt][test] Apply CFLAGS from sanitizer_common to all tests #120798

aaronpuchert · 2024-12-20T21:44:19Z

Sanitizer-specific tests don't use the sanitizer_common flags, but the issues they address probably also apply to the individual sanitizers.

This was observed in #119071: moving a test from sanitizer_common to msan broke it in builds with CMAKE_SYSROOT set, because the --sysroot argument was no longer applied to the test.

llvmbot · 2024-12-20T21:44:59Z

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Aaron Puchert (aaronpuchert)

Changes

Sanitizer-specific tests don't use the sanitizer_common flags, but the issues they address probably also apply to the individual sanitizers.

This was observed in #119071: moving a test from sanitizer_common to msan broke it in builds with CMAKE_SYSROOT set, because the --sysroot argument was no longer applied to the test.

Full diff: https://github.com/llvm/llvm-project/pull/120798.diff

2 Files Affected:

(modified) compiler-rt/cmake/config-ix.cmake (+8)
(modified) compiler-rt/test/sanitizer_common/CMakeLists.txt (-11)

diff --git a/compiler-rt/cmake/config-ix.cmake b/compiler-rt/cmake/config-ix.cmake
index 6d52eecc9a91fe..09c15fdabfd09c 100644
--- a/compiler-rt/cmake/config-ix.cmake
+++ b/compiler-rt/cmake/config-ix.cmake
@@ -307,6 +307,14 @@ macro(get_test_cc_for_arch arch cc_out cflags_out)
     if(APPLE)
       list(APPEND ${cflags_out} ${DARWIN_osx_CFLAGS})
     endif()
+    if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
+      # ARM on Linux might use the slow unwinder as default and the unwind table
+      # is required to get a complete stacktrace.
+      list(APPEND ${cflags_out} -funwind-tables)
+      if(CMAKE_SYSROOT)
+        list(APPEND ${cflags_out} "--sysroot=${CMAKE_SYSROOT}")
+      endif()
+    endif()
     string(REPLACE ";" " " ${cflags_out} "${${cflags_out}}")
   endif()
 endmacro()
diff --git a/compiler-rt/test/sanitizer_common/CMakeLists.txt b/compiler-rt/test/sanitizer_common/CMakeLists.txt
index 615666676f57ac..b044b9c6c3e088 100644
--- a/compiler-rt/test/sanitizer_common/CMakeLists.txt
+++ b/compiler-rt/test/sanitizer_common/CMakeLists.txt
@@ -73,17 +73,6 @@ foreach(tool ${SUPPORTED_TOOLS})
     get_test_cc_for_arch(${arch} SANITIZER_COMMON_TEST_TARGET_CC SANITIZER_COMMON_TEST_TARGET_CFLAGS)
     set(CONFIG_NAME ${tool}-${arch}-${OS_NAME})
 
-    # ARM on Linux might use the slow unwinder as default and the unwind table is
-    # required to get a complete stacktrace.
-    if ("${CMAKE_SYSTEM_NAME}" STREQUAL "Linux" AND NOT ANDROID)
-      list(APPEND SANITIZER_COMMON_TEST_TARGET_CFLAGS -funwind-tables)
-      if(CMAKE_SYSROOT)
-        list(APPEND SANITIZER_COMMON_TEST_TARGET_CFLAGS "--sysroot=${CMAKE_SYSROOT}")
-      endif()
-      string(REPLACE ";" " " SANITIZER_COMMON_TEST_TARGET_CFLAGS
-                             "${SANITIZER_COMMON_TEST_TARGET_CFLAGS}")
-    endif()
-
     configure_lit_site_cfg(
       ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
       ${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME}/lit.site.cfg.py)

aaronpuchert · 2024-12-20T21:54:26Z

@nico or @alanzhao1: could you test this together with #119071 relanded? Does that solve the issue (and not create new issues)?

aaronpuchert · 2025-02-26T16:53:41Z

Ping @nico / @alanzhao1. Since you reverted #119071 and you were the only ones to run into this issue, I'd appreciate if you could test this. Otherwise I'll have to land both changes and then see you revert them again if something doesn't work.

I've managed to somewhat reproduce this with a different sysroot, but the problem here arises due to the combination of sysroot and base image, and I can't exactly produce the setting that you have.

I'll also discuss offering both symbol versions, but I don't have a good idea for that yet.

compiler-rt/cmake/config-ix.cmake

vitalybuka

I suspect new code needs to be outside of
"if(ANDROID OR (NOT APPLE AND ${arch} MATCHES "arm|aarch64|riscv32|riscv64"))"

aaronpuchert · 2025-05-07T09:36:54Z

I've not changed anything about when -funwind-tables is applied, this is just moving the flags to apply to all tests. (All that use this function.)

Sanitizer-specific tests don't use the sanitizer_common flags, but the issues they address probably also apply to the individual sanitizers. This was observed in llvm#119071: moving a test from sanitizer_common to msan broke it in builds with CMAKE_SYSROOT set, because the --sysroot argument was no longer applied to the test.

gulfemsavrun · 2025-08-05T14:35:13Z

We started seeing a test failure after this patch in our toolchain builders:

FAIL: AddressSanitizer-x86_64-linux-dynamic :: TestCases/Linux/global-overflow-bfd.cpp (954 of 17470)
******************** TEST 'AddressSanitizer-x86_64-linux-dynamic :: TestCases/Linux/global-overflow-bfd.cpp' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
/b/s/w/ir/x/w/llvm_build/./bin/clang  --driver-mode=g++ -fsanitize=address -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only   -funwind-tables --sysroot=/b/s/w/ir/x/w/cipd/linux  -shared-libasan -fuse-ld=bfd -Wl,-gc-sections -ffunction-sections -fdata-sections -O0 /b/s/w/ir/x/w/llvm-llvm-project/compiler-rt/test/asan/TestCases/Linux/global-overflow-bfd.cpp -o /b/s/w/ir/x/w/llvm_build/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/compiler-rt/test/asan/X86_64LinuxDynamicConfig/TestCases/Linux/Output/global-overflow-bfd.cpp.tmp && not  /b/s/w/ir/x/w/llvm_build/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/compiler-rt/test/asan/X86_64LinuxDynamicConfig/TestCases/Linux/Output/global-overflow-bfd.cpp.tmp 2>&1 | FileCheck /b/s/w/ir/x/w/llvm-llvm-project/compiler-rt/test/asan/TestCases/Linux/global-overflow-bfd.cpp # RUN: at line 3
+ /b/s/w/ir/x/w/llvm_build/./bin/clang --driver-mode=g++ -fsanitize=address -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only -funwind-tables --sysroot=/b/s/w/ir/x/w/cipd/linux -shared-libasan -fuse-ld=bfd -Wl,-gc-sections -ffunction-sections -fdata-sections -O0 /b/s/w/ir/x/w/llvm-llvm-project/compiler-rt/test/asan/TestCases/Linux/global-overflow-bfd.cpp -o /b/s/w/ir/x/w/llvm_build/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/compiler-rt/test/asan/X86_64LinuxDynamicConfig/TestCases/Linux/Output/global-overflow-bfd.cpp.tmp
/b/s/w/ir/x/w/cipd/binutils-gdb/bin/ld.bfd: warning: libdl.so.2, needed by /b/s/w/ir/x/w/llvm_build/lib/clang/22/lib/x86_64-unknown-linux-gnu/libclang_rt.asan.so, not found (try using -rpath or -rpath-link)
/b/s/w/ir/x/w/cipd/binutils-gdb/bin/ld.bfd: warning: librt.so.1, needed by /b/s/w/ir/x/w/llvm_build/lib/clang/22/lib/x86_64-unknown-linux-gnu/libclang_rt.asan.so, not found (try using -rpath or -rpath-link)
/b/s/w/ir/x/w/cipd/binutils-gdb/bin/ld.bfd: warning: libpthread.so.0, needed by /b/s/w/ir/x/w/llvm_build/lib/clang/22/lib/x86_64-unknown-linux-gnu/libclang_rt.asan.so, not found (try using -rpath or -rpath-link)
/b/s/w/ir/x/w/cipd/binutils-gdb/bin/ld.bfd: /b/s/w/ir/x/w/llvm_build/lib/clang/22/lib/x86_64-unknown-linux-gnu/libclang_rt.asan.so: undefined reference to `pthread_setspecific@GLIBC_2.2.5'
/b/s/w/ir/x/w/cipd/binutils-gdb/bin/ld.bfd: /b/s/w/ir/x/w/llvm_build/lib/clang/22/lib/x86_64-unknown-linux-gnu/libclang_rt.asan.so: undefined reference to `pthread_rwlock_rdlock@GLIBC_2.2.5'
/b/s/w/ir/x/w/cipd/binutils-gdb/bin/ld.bfd: /b/s/w/ir/x/w/llvm_build/lib/clang/22/lib/x86_64-unknown-linux-gnu/libclang_rt.asan.so: undefined reference to `pthread_getspecific@GLIBC_2.2.5'
/b/s/w/ir/x/w/cipd/binutils-gdb/bin/ld.bfd: /b/s/w/ir/x/w/llvm_build/lib/clang/22/lib/x86_64-unknown-linux-gnu/libclang_rt.asan.so: undefined reference to `dlvsym@GLIBC_2.2.5'
/b/s/w/ir/x/w/cipd/binutils-gdb/bin/ld.bfd: /b/s/w/ir/x/w/llvm_build/lib/clang/22/lib/x86_64-unknown-linux-gnu/libclang_rt.asan.so: undefined reference to `pthread_key_create@GLIBC_2.2.5'
/b/s/w/ir/x/w/cipd/binutils-gdb/bin/ld.bfd: /b/s/w/ir/x/w/llvm_build/lib/clang/22/lib/x86_64-unknown-linux-gnu/libclang_rt.asan.so: undefined reference to `dladdr@GLIBC_2.2.5'
/b/s/w/ir/x/w/cipd/binutils-gdb/bin/ld.bfd: /b/s/w/ir/x/w/llvm_build/lib/clang/22/lib/x86_64-unknown-linux-gnu/libclang_rt.asan.so: undefined reference to `pthread_rwlock_unlock@GLIBC_2.2.5'
/b/s/w/ir/x/w/cipd/binutils-gdb/bin/ld.bfd: /b/s/w/ir/x/w/llvm_build/lib/clang/22/lib/x86_64-unknown-linux-gnu/libclang_rt.asan.so: undefined reference to `dlsym@GLIBC_2.2.5'
/b/s/w/ir/x/w/cipd/binutils-gdb/bin/ld.bfd: /b/s/w/ir/x/w/llvm_build/lib/clang/22/lib/x86_64-unknown-linux-gnu/libclang_rt.asan.so: undefined reference to `pthread_getattr_np@GLIBC_2.2.5'
/b/s/w/ir/x/w/cipd/binutils-gdb/bin/ld.bfd: /b/s/w/ir/x/w/llvm_build/lib/clang/22/lib/x86_64-unknown-linux-gnu/libclang_rt.asan.so: undefined reference to `pthread_attr_setstacksize@GLIBC_2.2.5'
/b/s/w/ir/x/w/cipd/binutils-gdb/bin/ld.bfd: /b/s/w/ir/x/w/llvm_build/lib/clang/22/lib/x86_64-unknown-linux-gnu/libclang_rt.asan.so: undefined reference to `pthread_rwlock_wrlock@GLIBC_2.2.5'
clang: error: linker command failed with exit code 1 (use -v to see invocation)

--

https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8707434189452310849/overview

… tests" (#152177) Reverts #120798 because it caused an AddressSanitizer test breakage in global-overflow-bfd.cpp: https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8707434189452310849/overview

…mmon to all tests" (#152177) Reverts llvm/llvm-project#120798 because it caused an AddressSanitizer test breakage in global-overflow-bfd.cpp: https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8707434189452310849/overview

aaronpuchert · 2025-08-06T12:40:24Z

@gulfemsavrun, can you explain how this impacts your build configuration? Do you use CMAKE_SYSROOT? I don't see any in the log, so I presume not? So I wonder what changed here.

As a side note, it's really annoying that whatever I do in compiler-rt, I get my change reverted by Google because it breaks some obscure build configuration that is not covered by the build bots. This change is already trying to fix an issue that caused a previous revert, and as you can see, I got zero feedback for half a year from the people who reverted my original change. (The change was arguably correct, but --sysroot was applied only to some tests, which caused a failure with certain sysroots.)

aaronpuchert · 2025-08-06T12:47:47Z

Sorry, I must have overlooked it. The log has --sysroot.

But is that from CMAKE_SYSROOT? Then I wonder where "undefined reference to" is coming from, since the runtime should have been build against the same sysroot.

aaronpuchert · 2025-08-06T13:22:34Z

In general, I don't know what the desired semantics of CMAKE_SYSROOT is. It's currently handled inconsistently, and it seems that Google's builds rely on that inconsistency.

It should probably apply to the build of LLVM (+ components) itself.
Should it apply to building the tests?
Should it apply to running the tests?

The answer to the second question was "sometimes" and I've tried to change it to "always" here. This should fix the issue after #119071, where the sysroot was older, but here it seems like the sysroot is newer? And then we would also need to run the tests against the sysroot? But maybe this is going to break another build, where the sysroot isn't actual meant for running stuff against it?

aaronpuchert · 2025-09-06T16:39:57Z

This is the only test with sanitizer runtimes and -fuse-ld=bfd. The build itself is configured with -DCMAKE_LINKER=/b/s/w/ir/x/w/cipd/bin/ld.lld. So maybe ld.bfd is just generally unable to find libraries in that setup? Could you check that the sysroot is correctly set up?

llvmbot added compiler-rt compiler-rt:sanitizer labels Dec 20, 2024

aaronpuchert requested a review from vitalybuka March 4, 2025 22:45

vitalybuka reviewed Mar 9, 2025

View reviewed changes

compiler-rt/cmake/config-ix.cmake Outdated Show resolved Hide resolved

vitalybuka requested changes Mar 9, 2025

View reviewed changes

vitalybuka requested review from petrhosek and vitalybuka May 6, 2025 23:26

vitalybuka approved these changes May 7, 2025

View reviewed changes

aaronpuchert force-pushed the sanitizer-common-test-flags branch from 288b004 to faea58a Compare May 7, 2025 09:26

aaronpuchert force-pushed the sanitizer-common-test-flags branch from faea58a to 73da76d Compare August 4, 2025 20:11

aaronpuchert merged commit b757bc8 into llvm:main Aug 4, 2025
9 checks passed

aaronpuchert deleted the sanitizer-common-test-flags branch August 4, 2025 20:39

gulfemsavrun mentioned this pull request Aug 5, 2025

Revert "[compiler-rt][test] Apply CFLAGS from sanitizer_common to all tests" #152177

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[compiler-rt][test] Apply CFLAGS from sanitizer_common to all tests #120798

[compiler-rt][test] Apply CFLAGS from sanitizer_common to all tests #120798

Uh oh!

aaronpuchert commented Dec 20, 2024

Uh oh!

llvmbot commented Dec 20, 2024

Uh oh!

aaronpuchert commented Dec 20, 2024

Uh oh!

aaronpuchert commented Feb 26, 2025

Uh oh!

Uh oh!

vitalybuka left a comment

Uh oh!

aaronpuchert commented May 7, 2025

Uh oh!

Uh oh!

gulfemsavrun commented Aug 5, 2025

Uh oh!

aaronpuchert commented Aug 6, 2025 •

edited

Loading

Uh oh!

aaronpuchert commented Aug 6, 2025

Uh oh!

aaronpuchert commented Aug 6, 2025

Uh oh!

aaronpuchert commented Sep 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[compiler-rt][test] Apply CFLAGS from sanitizer_common to all tests #120798

[compiler-rt][test] Apply CFLAGS from sanitizer_common to all tests #120798

Uh oh!

Conversation

aaronpuchert commented Dec 20, 2024

Uh oh!

llvmbot commented Dec 20, 2024

Uh oh!

aaronpuchert commented Dec 20, 2024

Uh oh!

aaronpuchert commented Feb 26, 2025

Uh oh!

Uh oh!

vitalybuka left a comment

Choose a reason for hiding this comment

Uh oh!

aaronpuchert commented May 7, 2025

Uh oh!

Uh oh!

gulfemsavrun commented Aug 5, 2025

Uh oh!

aaronpuchert commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aaronpuchert commented Aug 6, 2025

Uh oh!

aaronpuchert commented Aug 6, 2025

Uh oh!

aaronpuchert commented Sep 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aaronpuchert commented Aug 6, 2025 •

edited

Loading